feat: disable product cookie when no targeting is true by jaissica12 · Pull Request #1071 · mParticle/mparticle-web-sdk

jaissica12 · 2025-09-16T20:57:29Z

Instructions

PR target branch should be against development
PR title name should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-title-check.yml
PR branch prefix should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-branch-check-name.yml

Summary

Updated persistence logic for products local storage:
- Read: not read/consumed when noTargeting is true
- Create: not created when noTargeting is true
- Update: not updated when noTargeting is true; any existing products storage is also not removed
Added jest tests and integration tests

Testing Plan

Was this tested locally? If not, explain why.
{explain how this has been tested, and what, if any, additional testing should be done}

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

Closes https://rokt.atlassian.net/browse/SDKE-92

alexs-mparticle · 2025-09-18T19:16:18Z

+
+    it('should update Products localStorage when noTargeting is false by default', () => {
+        const product = { Name: 'iphone', Sku: 'iphonesku', Price: 599, Quantity: 1 };
+        mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } } as any);


Suggested change

mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } } as any);

mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } });

alexs-mparticle · 2025-09-18T19:16:29Z

+    it('should NOT update Products localStorage when noTargeting is true', () => {
+        mpInstance._Store.setNoTargeting(true);
+        const product = { Name: 'iphone', Sku: 'iphonesku', Price: 599, Quantity: 1 };
+        mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } } as any);


Suggested change

mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } } as any);

mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } });

alexs-mparticle · 2025-09-18T19:16:38Z

+    it('should update Products localStorage when noTargeting is false', () => {
+        mpInstance._Store.setNoTargeting(false);
+        const product = { Name: 'iphone', Sku: 'iphonesku', Price: 599, Quantity: 1 };
+        mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } } as any);


Suggested change

mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } } as any);

mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } });

rmi22186 · 2025-09-18T22:08:23Z

    this.setCartProducts = function(allProducts) {
-        if (!mpInstance._Store.isLocalStorageAvailable) {
+        if (
+            !mpInstance._Store.isLocalStorageAvailable &&


This should be OR, not AND. If local storage isn't available, or if getNoTargeting returns true, then don't set the cart products

Suggested change

!mpInstance._Store.isLocalStorageAvailable &&

!mpInstance._Store.isLocalStorageAvailable || mpInstance._Store.getNoTargeting

rmi22186 · 2025-09-18T22:09:43Z

+        mParticle._resetForTests(MPConfig);
+    });
+
+    it('should update Products localStorage when noTargeting is false by default', () => {


Suggested change

it('should update Products localStorage when noTargeting is false by default', () => {

it('should save products to localStorage when noTargeting is false by default', () => {

rmi22186 · 2025-09-18T22:10:02Z

+        expect(mpInstance._Persistence.getCartProducts(testMPID).length).toBe(1);
+    });
+
+    it('should NOT update Products localStorage when noTargeting is true', () => {


Suggested change

it('should NOT update Products localStorage when noTargeting is true', () => {

it('should NOT save products to localStorage when noTargeting is true', () => {

rmi22186 · 2025-09-18T22:10:18Z

+        expect(mpInstance._Persistence.getCartProducts(testMPID)).toEqual([]);
+    });
+
+    it('should update Products localStorage when noTargeting is false', () => {


Suggested change

it('should update Products localStorage when noTargeting is false', () => {

it('should save products to localStorage when noTargeting is false', () => {

rmi22186 · 2025-09-18T22:11:39Z

+        }); 
+
+        it('should not save products LS when noTargeting is true', async () => {
+            mParticle.config.noTargeting = true;


I'm surprised this is working.

Per the docs, this should be:

Suggested change

mParticle.config.noTargeting = true;

mParticle.config.launcherOptions = {noTargeting: true};

@rmi22186 Currently noFunctional / noTargeting is only read from top‑level mParticle.config during init, not from config.launcherOptions that is why mParticle.config.noTargeting = true; works. To use mParticle.config.launcherOptions = {noTargeting: true}; we need to update store to source privacy flags from config.launcherOptions.

Now that you have updated the noTargeting and noFunctional to pull from config.launcherOptions, this will need to be updated

rmi22186 · 2025-09-18T22:11:53Z

+            mParticle._resetForTests(MPConfig);
+        }); 
+
+        it('should not save products LS when noTargeting is true', async () => {


Suggested change

it('should not save products LS when noTargeting is true', async () => {

it('should not save products to LS when noTargeting is true', async () => {

rmi22186 · 2025-09-18T22:12:01Z

+            expect(localStorageProducts).to.not.be.ok;
+        });
+
+        it('should save products LS when noTargeting is false', async () => {


Suggested change

it('should save products LS when noTargeting is false', async () => {

it('should save products to LS when noTargeting is false', async () => {

rmi22186 · 2025-09-18T22:12:20Z

+        });
+
+        it('should save products LS when noTargeting is false', async () => {
+            mParticle.config.noTargeting = false;


Suggested change

mParticle.config.noTargeting = false;

mParticle.config.launcherOptions = {noTargeting: false};

rmi22186 · 2025-09-18T22:12:46Z

+            expect(localStorageProducts).to.be.ok;
+        });
+
+        it('should save products LS when noTargeting is false by default', async () => {


Suggested change

it('should save products LS when noTargeting is false by default', async () => {

it('should save products to LS when noTargeting is false by default', async () => {

rmi22186

minor nits. also needs to be rebased against #1063 and logic need to be updated using mpInstance._Store.getPrivacyFlag

…isable-product-cookie-when-noTargeting-is-true

rmi22186

There are changes to persistnece.js in both
https://github.com/mParticle/mparticle-web-sdk/pull/1070/files and
https://github.com/mParticle/mparticle-web-sdk/pull/1071/files

I feel like it's going to create a merging/rebasing nightmare once 1070 is merged, or 1071 is merged first. I think you need to either combine 1070 and 1071 into a single PR, or use 1070 as what you are PR-ing into instead of development for this branch.

jaissica12 · 2025-09-25T20:44:10Z

There are changes to persistnece.js in both https://github.com/mParticle/mparticle-web-sdk/pull/1070/files and https://github.com/mParticle/mparticle-web-sdk/pull/1071/files

I feel like it's going to create a merging/rebasing nightmare once 1070 is merged, or 1071 is merged first. I think you need to either combine 1070 and 1071 into a single PR, or use 1070 as what you are PR-ing into instead of development for this branch.

I agree! I have changed the target for PR to 1070. Once this PR is approved I'll merge it into 1070 and then:

Clean up persistence logic wherever its needed.
Merge the tests for Persistence into one file (persistence.spec.ts)

rmi22186 · 2025-09-26T13:23:33Z

+        if (mpInstance._Store.getPrivacyFlag('Products')) {
+            return;
+        }
        if (mpid) {
            allLocalStorageProducts = allLocalStorageProducts || {};
            allLocalStorageProducts[mpid] = currentUserProducts;
            try {
-                window.localStorage.setItem(
-                    encodeURIComponent(mpInstance._Store.prodStorageName),
-                    Base64.encode(JSON.stringify(allLocalStorageProducts))
-                );
+                if (!mpInstance._Store.getPrivacyFlag('Products')) {
+                    window.localStorage.setItem(
+                        encodeURIComponent(mpInstance._Store.prodStorageName),
+                        Base64.encode(JSON.stringify(allLocalStorageProducts))
+                    );
+                }


Is the if check for 368 necessary here? Since you have 361-363 already returning based on the Products privacy flag, I don't think you need to check for the inverse of 361 on line 368.

I agree, its not needed here since we're already returning at line 362

rmi22186

minor nits

alexs-mparticle · 2025-09-26T13:39:47Z

+                    window.localStorage.setItem(
+                        encodeURIComponent(mpInstance._Store.prodStorageName),
+                        Base64.encode(JSON.stringify(allLocalStorageProducts))
+                    );


I would also clean up the original code a bit so that i's more readable:

Suggested change

window.localStorage.setItem(

encodeURIComponent(mpInstance._Store.prodStorageName),

Base64.encode(JSON.stringify(allLocalStorageProducts))

);

const encodedKey = encodeURIComponent(mpInstance._Store.prodStorageName);

const encodedValue = Base64.encode(JSON.stringify(allLocalStorageProducts));

window.localStorage.setItem(encodedKey, encodedValue);

alexs-mparticle · 2025-09-26T13:43:31Z

+          const product = { Name: 'iphone', Sku: 'iphonesku', Price: 599, Quantity: 1 };
+          mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } } as unknown as Product[]);
+          expect(mpInstance._Persistence.getCartProducts(testMPID).length).toBe(1);


I think instead of trying to coerce the Type, let's see if we can use a Partial so we can maintain type safety.

Suggested change

const product = { Name: 'iphone', Sku: 'iphonesku', Price: 599, Quantity: 1 };

mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } } as unknown as Product[]);

expect(mpInstance._Persistence.getCartProducts(testMPID).length).toBe(1);

const product: Partial<Product> = { Name: 'iphone', Sku: 'iphonesku', Price: 599, Quantity: 1 };

mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } });

expect(mpInstance._Persistence.getCartProducts(testMPID).length).toBe(1);

@alexs-mparticle I’ve incorporated Partial<Product> for the item, but I’m still hitting the TypeScript error since setCartProducts is typed as Product[] while the runtime path expects a per‑MPID map. That’s why I had to coerce the call to match the actual shape earlier as well. If there’s a better approach or a preferred workaround to avoid the cast while keeping the test on the real write path, please advise and I’ll update.

sonarqubecloud · 2025-09-26T14:29:24Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

jaissica12 · 2025-09-26T21:05:02Z

Closing this PR since we are removing setCartProducts and getCartProducts

jaissica12 added 2 commits September 16, 2025 11:39

feat: disable product cookie when persistence is set and updated test

b7230d6

test: added jest tests and updated test content

b14099e

jaissica12 requested review from alexs-mparticle and rmi22186 September 16, 2025 20:57

alexs-mparticle requested changes Sep 18, 2025

View reviewed changes

rmi22186 reviewed Sep 18, 2025

View reviewed changes

Comment thread src/persistence.js

rmi22186 reviewed Sep 18, 2025

View reviewed changes

jaissica12 added 4 commits September 19, 2025 13:03

Merge remote-tracking branch 'origin/development' into feat/SDKE-92-d…

bbe4f08

…isable-product-cookie-when-noTargeting-is-true

refactor: updated getPrivacyFlag in persistence and updated test content

507b3e5

test: updated jest test for persistence

9f062ca

test: updated jest test for persistence failure

d09d4c9

jaissica12 requested review from alexs-mparticle and rmi22186 September 19, 2025 20:16

jaissica12 added 2 commits September 23, 2025 13:49

Merge remote-tracking branch 'origin/development' into feat/SDKE-92-d…

273e170

…isable-product-cookie-when-noTargeting-is-true

test: source privacy flag from launcherOptions

0847f1e

rmi22186 requested changes Sep 25, 2025

View reviewed changes

jaissica12 changed the base branch from development to feat/SDKE-91-disable-session-cookie-localStorage-when-noFunctional-is-set September 25, 2025 12:52

jaissica12 mentioned this pull request Sep 25, 2025

feat: disable session cookie/localStorage when noFunctional is set #1070

Merged

1 task

test: move/rename test to persistence.products.spec

bfcdb2b

jaissica12 requested a review from rmi22186 September 25, 2025 21:12

rmi22186 reviewed Sep 26, 2025

View reviewed changes

rmi22186 requested changes Sep 26, 2025

View reviewed changes

alexs-mparticle requested changes Sep 26, 2025

View reviewed changes

jaissica12 added 2 commits September 26, 2025 09:44

chore: removed redundant if block

6c0c0c8

test: updated products persistence test

95d8fd8

jaissica12 closed this Sep 26, 2025

	mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } } as any);
	mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } });

	!mpInstance._Store.isLocalStorageAvailable &&
	!mpInstance._Store.isLocalStorageAvailable \|\| mpInstance._Store.getNoTargeting

	it('should update Products localStorage when noTargeting is false by default', () => {
	it('should save products to localStorage when noTargeting is false by default', () => {

	it('should NOT update Products localStorage when noTargeting is true', () => {
	it('should NOT save products to localStorage when noTargeting is true', () => {

	mParticle.config.noTargeting = true;
	mParticle.config.launcherOptions = {noTargeting: true};

	it('should not save products LS when noTargeting is true', async () => {
	it('should not save products to LS when noTargeting is true', async () => {

	it('should save products LS when noTargeting is false', async () => {
	it('should save products to LS when noTargeting is false', async () => {

	mParticle.config.noTargeting = false;
	mParticle.config.launcherOptions = {noTargeting: false};

Uh oh!

Conversation

jaissica12 commented Sep 16, 2025

Instructions

Summary

Testing Plan

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rmi22186 left a comment

Choose a reason for hiding this comment

Uh oh!

rmi22186 left a comment

Choose a reason for hiding this comment

Uh oh!

jaissica12 commented Sep 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rmi22186 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Sep 26, 2025

Quality Gate passed

Uh oh!

jaissica12 commented Sep 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants